Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass header flag to Renderer.tablerow method #818

Closed
wants to merge 1 commit into from

Conversation

ksheedlo
Copy link

When styling tables out of a custom renderer subclass, it's useful to
know if the tablerow is in the header or body. Tablecells already get a
header flag that is set if the tablecell is in the header. This change
adds the flag to tablerows.

When styling tables out of a custom renderer subclass, it's useful to
know if the tablerow is in the header or body. Tablecells already get a
header flag that is set if the tablecell is in the header. This change
adds the flag to tablerows.
@joshbruce
Copy link
Member

Closing due to missing tests - going to flag with #981 for future reference as we focus on correcting known issues.

@joshbruce joshbruce closed this Jan 21, 2018
@Feder1co5oave
Copy link
Contributor

Feder1co5oave commented Jan 22, 2018

This specific pr is not about parsing, so there's nothing to test here.
I can't see a practical use for this, but I guess it would be nice to have for someone. I'd flag it for the 0.6.0 milestone

Also, this is about (gfm) tables, not headings.

@joshbruce joshbruce added this to the 0.6.0 - Improve developer experience milestone Jan 22, 2018
@joshbruce
Copy link
Member

Is this a GFM table extension - think all the table stuff in GFM is an extension; so, definitely not an immediate need.

@Feder1co5oave
Copy link
Contributor

Feder1co5oave commented Jan 24, 2018

Tables are supported in gfm mode only, and yes, the whole gfm is an extension to markdown, but not github-specific. gfm is on by default.

Must remember to adjust the tablerow renderer and put in the additional parameter, for sake of "documentation".

Now that I think about it, this should come in helpful when extending marked, so maybe 0.5.0 would be better?

@joshbruce
Copy link
Member

It's possible. I think what I meant when asking if it was an extension is the GFM spec has certain concepts that are explicitly labeled as extensions - not part of their base specification, which is an extension of the original Markdown.py: https://github.github.com/gfm/

Task lists, tables, and autolinks, for example, are explicitly called out as extensions. Think the focus for us right now is on the base specs for CommonMark and GFM - not extensions - at least according to #956.

Think you're probably right about the possibility of helping to inform the 0.5.0 work. Changing.

@joshbruce joshbruce modified the milestones: 0.6.0 - Improve developer experience, 0.5.0 - Architecture and extensibility Jan 24, 2018
@ksheedlo
Copy link
Author

I can't see a practical use for this

I'm using it in my fork to put a thicker border under the thead > tr, like this:

screen shot 2018-01-24 at 2 47 44 pm

I suppose I could hack it by setting that border on the th elements instead, but doing it this way seems nicer.

@Feder1co5oave
Copy link
Contributor

@joshbruce there is no "gfm markdown", gfm is commonmark + gfm extensions (i.e. tables, task lists, autolinks, etc.)

@joshbruce
Copy link
Member

@Feder1co5oave: Interesting. Did not realize. Thought they were two separate specs - makes life easier then. :)

@ksheedlo: Seems odd to ask Marked to do that - tightly couples markup and style, if I'm understanding the resulting markup properly. Can you post the markup that gives you that result?

@joshbruce
Copy link
Member

joshbruce commented Jan 25, 2018

@Feder1co5oave: #746 - As we consider architecture and extensibility we should start considering what we want Marked to take on beyond the specs a bit more. If we physically separate the parser and whatnot as desicribed by @Nalinc it might become more obvious on how to override/extned; thereby, opening the door to folks to do things like what's in #1030 or here instead of asking Marked to do it - and for us to maintain it. Really don't want to end up in the same situation again where none of us really have time to maintain Marked because it is taking on too much, you know?

Tagging in @UziTech and @KostyaTretyak

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants